daemon: skip querying currentConfig if possible#312
daemon: skip querying currentConfig if possible#312openshift-merge-robot merged 2 commits intoopenshift:masterfrom
Conversation
|
I was looking at a similar patch recently, but I was thinking more like |
|
There's all these layers of "update/triggerUpdate*" functions that get into the "onceFrom" stuff and I get a bit confused by them...but the lowest level seems to be |
|
Yeah, I think that'd work. Though it's a bit more involved than I wanted to go given the current freeze. Whereas the patch as written is pretty easy to reason through. Let's enhance it later? Your diff did make me notice something else though. I pushed another minor commit for that! ⬆️ |
b6d739b to
7bab9fe
Compare
The same way we pass the specific desiredConfig we "locked on" when updating, also similarly pass the currentConfig. This is a minor optimization so we avoid querying the cluster again for information we already have, but it does also make the code more resilient to humans meddling with node annotations. Related: openshift#301
We should drain the node if we're *not* in one-shot mode. The previous conditional still worked because "" is not a valid path, but it muddles the intent.
7bab9fe to
fde547b
Compare
|
Infra flake /retest |
|
/retest |
|
Terraform flake /retest |
ashcrow
left a comment
There was a problem hiding this comment.
Deferring to a second review to merge.
|
/approve |
|
Monitor flake: /retest |
|
/lgtm This will help a bit for #273 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ashcrow, cgwalters, jlebon The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
|
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
bump(*) to v1.19.0-rc.3
The same way we pass the specific desiredConfig we "locked on" when
updating, also similarly pass the currentConfig. This is a minor
optimization so we avoid querying the cluster again for information we
already have, but it does also make the code more resilient to humans
meddling with node annotations.
Requires: #310